Skip to content

fix: allowing unknown solvers through mirror solved-issue pipeline#87

Open
jonathanchang31 wants to merge 1 commit into
entrius:testfrom
jonathanchang31:fix/missing-pr-author-null
Open

fix: allowing unknown solvers through mirror solved-issue pipeline#87
jonathanchang31 wants to merge 1 commit into
entrius:testfrom
jonathanchang31:fix/missing-pr-author-null

Conversation

@jonathanchang31
Copy link
Copy Markdown

@jonathanchang31 jonathanchang31 commented May 13, 2026

Summary

This change fixes a scoring-integrity bug where backfill stored missing PR author IDs as empty strings ('') instead of NULL. Because solver selection only filtered IS NOT NULL, unknown solver identities leaked into solving_pr.

Change Type (select all)

  • Bug fix
  • Data integrity fix
  • API response correctness fix
  • Backfill pipeline correction
  • DB data normalization script
  • Breaking change
  • New feature

Root Cause

  • Backfill PR upsert used:
    • authorGithubId: String(pr.author?.databaseId ?? "")
  • Empty string passed solver guard:
    • sp.author_github_id IS NOT NULL

Result: unknown solver IDs ('') were treated as present and returned in solved-issue pipeline.

Real Behavior Proof

Before

  • Missing PR author in backfill -> author_github_id = ''
  • Solver subquery accepted it (IS NOT NULL true)
  • API could emit solved issues with unknown/blank solver identity

After

  • Missing PR author in backfill -> author_github_id = NULL
  • Solver subquery rejects both null and blank IDs
  • Unknown solvers are excluded from solving_pr as intended

Checklist

  • Fix implemented in backfill PR ingestion path
  • Solver SQL guard hardened for blank values
  • Legacy normalization SQL added
  • Patch re-validated by code inspection
  • Migration applied in target DB environment
  • Tests run in CI/local

@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label May 13, 2026
Copy link
Copy Markdown
Collaborator

@anderdc anderdc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope this down to the one-line read-time guard.

Keep:

  • miners.service.ts:158 adding AND BTRIM(sp.author_github_id) <> '' — that single line filters out both legacy '' rows and any future blank writes, fully resolving the API-surface leak.

Drop:

  • The TS change in github-fetcher.service.ts — once the read-time guard exists, the column value isn't user-visible whether it's '' or NULL, so writing NULL prospectively is cleanup without payoff.
  • packages/db/26_normalize_blank_pr_author_ids.sql — the guard makes legacy blanks invisible at read time; no schema-tree file or migration runbook needed.

Also drop the Fixes: reference (already removed from the description) — #85 was closed because the prescribed multi-step fix was over-scoped for the user-facing impact (phantom solver rows don't misroute scoring since registered miners can't have empty github_ids). The one-liner captures the win without the schema-tree change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants